-
Notifications
You must be signed in to change notification settings - Fork 851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Fixed overrideSndSeqNo() not clear buffer. #2753
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #2753 +/- ##
==========================================
- Coverage 67.51% 66.77% -0.75%
==========================================
Files 99 99
Lines 20166 20178 +12
==========================================
- Hits 13616 13473 -143
- Misses 6550 6705 +155
... and 9 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Well, ok, but why is everything dropped from the sender buffer instead of only sequences up to the scheduling sequence? |
Correct me if I"m wrong, the scheduling sequence should be the last seq in sender buffer? If no other IPE, "everything" should be same as "only sequences up to the scheduling sequence"? |
2a5bb72
to
2740eea
Compare
2740eea
to
397dc7c
Compare
Depends on which sender buffer. We are with backup groups here, but sender buffering is separate for a socket and for a group. Every socket still has its own sender buffering and there's an extra sender buffer for the group, which are stored when group sending happens. The sender buffer for an idle member socket is not filled on regular sending, only when the backup-activation process starts and packets are being resent from the group sender buffer. The problem is that as the idle link doesn't send packets, it also doesn't store anything in the socket's sender buffer and hence doesn't increase the sending sequence. This gets "tricked", that is, when ACK comes, the anchor sequence of the sender buffer must be shifted to the position of the active link. When the backup activation happens, all packets since the last ACK should be resent, and the sender buffer of the socket should be prepared for this by getting shifted. However, ACK handler and backup activation happen in different threads, so there's no guarantee that the sender buffer will have the right anchor sequence number at the moment of the backup activation. The code must be then prepared for even a worst case scenario, that is, when the anchor sequence of the socket's sender buffer has to be fixed before sending the first packet from the group's sender buffer. It is, however, predicted that the sequence can be shifted forwards, not backwards. The need of backward fixing should be impossible because the anchor sequence either stays unchanged and is then behind the last sent, or it is shifted together with ACK and is equal to the anchor sequence of all other sockets. Another small problem is that at the moment of backup activation there shouldn't be any packets in the sender buffers of idle member sockets. I don't exactly remember how it is implemented, but anyway: it should catch the empty buffer and only make sure that the anchor sequence of the sender buffer is equal to the last ACK received. ACK should cause dismissal of packets from the group sender buffer as well by obvious reasons - so there shouldn't be also a situation when the sequence number of the packet picked up from the group sender buffer predates the activated member socket's sender buffer's anchor sequence. |
Note the
IPE
log, the SCHEDULING sequence1540037828
is behind of EXTRACTION sequence1540037911
, because of theSND-DROP: %(1540037828-1540037910)
.In
sndDropTooLate()
, them_iSndLastAck
was mistakenly updated to1540037911
bym_pSndBuffer->dropLateData()
, so the issue is thatm_pSndBuffer
was not cleared byoverrideSndSeqNo()
in main-backup mode.